-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Remove unneeded swift_name attributes from Objective-C headers #5518
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Please feel free to comment on my kotlin style. It is not my primary language by a long shot. |
val privateAttribute: String? = property.getSwiftPrivateAttribute() | ||
if (privateAttribute != null) { | ||
declarationAttributes.add(privateAttribute) | ||
} else if (propertyName.objCName != propertyName.swiftName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this optimization lead to an issue like this: JetBrains/kotlin-native#2571?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack. Was unaware of that case. Updated with fix.
56f6ce2
to
c04ac24
Compare
Ignore for a moment. Fixing the AA cases. |
Partial fix for KT-81447 Native: Objective-C: Excessive 'swift_name' attributes being generated Removes swift_name attribute from declarations that would already have that swift name. Initially focusing on properties and and simple methods that have no parameters. The excessive use of swift_attributes creates large headers, and even larger pcm files generated from those headers.
c04ac24
to
08eccc7
Compare
AA cases all fixed. It's a big change, but not sure how I could really break it up unless you want properties/methods split out. |
descriptor.enumEntries.forEach { | ||
val entryName = namer.getEnumEntrySelector(it) | ||
val swiftName = namer.getEnumEntrySwiftName(it) | ||
val declarationAttrs = if (entryName != swiftName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Swift uses various rules when importing ObjectiveC declarations (some of which discussed here). This includes (but is not limited to) adding "is" to boolean properties, removing "with", "to", "for", "from" in certain contexts, removing redundant types, and stripping namespace prefixes. Adding swift_name attribute overrides this behavior.
Am I correct to assume that, if by some chance a kotlin declaration includes any of above, it's handling by swift may change?
If yes, this is a very important concern, since this MR might introduce breaking changes for people using Kotlin's ObjC-exported code from Swift through native bridging.
Partial fix for
KT-81447 Native: Objective-C: Excessive 'swift_name' attributes being generated
Removes swift_name attribute from declarations that would already have that swift name. Initially focusing on properties and and simple methods that have no parameters.
The excessive use of swift_attributes creates large headers, and even larger pcm files generated from those headers.